Skip to content

Feat/support html node WIP #57

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Feat/support html node WIP #57

wants to merge 2 commits into from

Conversation

d4rekanguok
Copy link

Hello! This is an attempt at supporting html node mentioned in #56.

I added rehype & hast-util-to-string to turn this tree

u('root', [
    u('heading', {depth: 1}, [
      u('text', {value: 'Hello '}),
      u('html', {value: '<code>World</code>'})
    ])
  ])

into this:

...
{
  type: 'paragraph',
  children: [{
    type: 'link',
    title: null,
    url: '#hello-world',  // before: '#hello-codeworldcode'
    children: [{ 
      type: 'text', 
      value: 'Hello World'  // before: 'Hello <code>World<code>'
    }] // before: ''
  }]
}
...

It'd be great if we could actually get back the inline html in table of contents, such as...

  children: [{ 
      type: 'text', 
      value: 'Hello ' 
    }, {
      type: 'html',
      value: '<code>World<code>',
    }]

... but I couldn't figure out a simple way to achieve this. Also, after looking at the tests I think this is not consistent with other use case (mdast-util-to-string flatten any sort of emphasis or link nodes). I also wonder if bringing in rehype is overkill for this.

I'd appreciate any thoughts on this & whether it should be supported, thank you!

@wooorm
Copy link
Member

wooorm commented Apr 25, 2019

@d4rekanguok Thanks for starting work on this, appreciated! I don’t like the approach you’re taking tho, as you’re including the whole of rehype which adds a huge browser load for many of our users, that humans around the world don’t deserve!

See also my comment in the issue before I saw your PR: #56 (comment)

@d4rekanguok
Copy link
Author

@wooorm thank you for the quick feedback! your point about the browser load is heard loud and clear, my bad. If this were still to be considered, I'll try a different route.

@wooorm wooorm mentioned this pull request Apr 25, 2019
Merged
@wooorm wooorm closed this in #58 May 13, 2019
wooorm added a commit that referenced this pull request May 13, 2019
@wooorm
Add support for phrasing content
6fbb123 
Previously, only the textual content of headings was used to generate the entries in the Table of Contents. This lead to a bug (or feature?) where HTML in headings made entries in the TOC appear with literal HTML.

mdast has a concept of static phrasing content, which includes all the things that can appear in links. Instead of using just the textual content of a heading, this commit introduces a new behaviour where any static phrasing content, such as HTML, inline code, and emphasis, are copied over into the Table of Contents. Interactive phrasing content (links, link references) are handled by replacing them with their children.

See: <https://github.com/syntax-tree/mdast#staticphrasingcontent>

Closes GH-56.
Closes GH-57.
Closes GH-58.

Reviewed-by: Christian Murphy <christian.murphy.42@gmail.com>
Reviewed-by: Jonathan Haines <jonno.haines@gmail.com>
@wooorm wooorm added 🗄 area/interface This affects the public interface 🙅 no/wontfix This is not (enough of) an issue for this project 🦋 type/enhancement This is great to have 🧑 semver/major This is a change labels Aug 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
🗄 area/interface This affects the public interface 🙅 no/wontfix This is not (enough of) an issue for this project 🧑 semver/major This is a change 🦋 type/enhancement This is great to have
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@d4rekanguok @wooorm